diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef64de6cd..8ffd444bdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -237,6 +237,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `DatabasePermission` + - Fixed `Equals()` method to compare both `State` and `Permission` properties. + Previously, the method incorrectly referenced a non-existent `Grant` property, + causing incorrect equality comparisons when instances had different `State` + values ([issue #2386](https://github.com/dsccommunity/SqlServerDsc/issues/2386)). + - Added `GetHashCode()` override to align with `Equals()` semantics, ensuring + consistent behavior when instances are used in hash-based collections + ([issue #2386](https://github.com/dsccommunity/SqlServerDsc/issues/2386)). +- `ServerPermission` + - Fixed `Equals()` method to compare both `State` and `Permission` properties. + Previously, the method incorrectly referenced a non-existent `Grant` property, + causing incorrect equality comparisons when instances had different `State` + values ([issue #2386](https://github.com/dsccommunity/SqlServerDsc/issues/2386)). + - Added `GetHashCode()` override to align with `Equals()` semantics, ensuring + consistent behavior when instances are used in hash-based collections + ([issue #2386](https://github.com/dsccommunity/SqlServerDsc/issues/2386)). +- Unit Tests + - Fixed misleading variable naming in ServerPermission.Tests.ps1 where + `$databasePermissionInstance` was used for `[ServerPermission]` objects. + All instances have been renamed to `$serverPermissionInstance`. - `Set-SqlDscServerPermission` - Fixed an issue where unspecified permission parameters would incorrectly revoke existing permissions. The command now only processes permission diff --git a/source/Classes/002.DatabasePermission.ps1 b/source/Classes/002.DatabasePermission.ps1 index 175e5360e6..471f771192 100644 --- a/source/Classes/002.DatabasePermission.ps1 +++ b/source/Classes/002.DatabasePermission.ps1 @@ -183,7 +183,7 @@ class DatabasePermission : IComparable, System.IEquatable[Object] if ($object -is $this.GetType()) { - if ($this.Grant -eq $object.Grant) + if ($this.State -eq $object.State) { if (-not (Compare-Object -ReferenceObject $this.Permission -DifferenceObject $object.Permission)) { @@ -195,6 +195,18 @@ class DatabasePermission : IComparable, System.IEquatable[Object] return $isEqual } + [System.Int32] GetHashCode() + { + [System.Int32] $hashCode = $this.State.GetHashCode() + + foreach ($permission in ($this.Permission | Sort-Object)) + { + $hashCode = $hashCode -bxor $permission.GetHashCode() + } + + return $hashCode + } + [System.Int32] CompareTo([Object] $object) { [System.Int32] $returnValue = 0 diff --git a/source/Classes/002.ServerPermission.ps1 b/source/Classes/002.ServerPermission.ps1 index 90febf7259..8cf88ac671 100644 --- a/source/Classes/002.ServerPermission.ps1 +++ b/source/Classes/002.ServerPermission.ps1 @@ -115,7 +115,7 @@ class ServerPermission : IComparable, System.IEquatable[Object] if ($object -is $this.GetType()) { - if ($this.Grant -eq $object.Grant) + if ($this.State -eq $object.State) { if (-not (Compare-Object -ReferenceObject $this.Permission -DifferenceObject $object.Permission)) { @@ -127,6 +127,18 @@ class ServerPermission : IComparable, System.IEquatable[Object] return $isEqual } + [System.Int32] GetHashCode() + { + [System.Int32] $hashCode = $this.State.GetHashCode() + + foreach ($permission in ($this.Permission | Sort-Object)) + { + $hashCode = $hashCode -bxor $permission.GetHashCode() + } + + return $hashCode + } + <# TODO: It was not possible to move this to a parent class. But since these are generic functions for DatabasePermission and ServerPermission we diff --git a/tests/Unit/Classes/DatabasePermission.Tests.ps1 b/tests/Unit/Classes/DatabasePermission.Tests.ps1 index 6a845c5646..3d15d94ab6 100644 --- a/tests/Unit/Classes/DatabasePermission.Tests.ps1 +++ b/tests/Unit/Classes/DatabasePermission.Tests.ps1 @@ -130,34 +130,64 @@ Describe 'DatabasePermission' -Tag 'DatabasePermission' { } Context 'When object has different value for property State' { - It 'Should instantiate two objects' { - $script:mockDatabasePermissionInstance1 = InModuleScope -ScriptBlock { - Set-StrictMode -Version 1.0 + Context 'When comparing Deny with Grant' { + It 'Should instantiate two objects' { + $script:mockDatabasePermissionInstance1 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 - $databasePermissionInstance = [DatabasePermission]::new() + $databasePermissionInstance = [DatabasePermission]::new() - $databasePermissionInstance.State = 'Deny' - $databasePermissionInstance.Permission = 'select' + $databasePermissionInstance.State = 'Deny' + $databasePermissionInstance.Permission = 'select' - return $databasePermissionInstance - } + return $databasePermissionInstance + } - $script:mockDatabasePermissionInstance2 = InModuleScope -ScriptBlock { - Set-StrictMode -Version 1.0 + $script:mockDatabasePermissionInstance2 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 - $databasePermissionInstance = [DatabasePermission]::new() + $databasePermissionInstance = [DatabasePermission]::new() - $databasePermissionInstance.State = 'Grant' - $databasePermissionInstance.Permission = 'select' + $databasePermissionInstance.State = 'Grant' + $databasePermissionInstance.Permission = 'select' - return $databasePermissionInstance + return $databasePermissionInstance + } + } + + It 'Should return $false' { + $mockDatabasePermissionInstance1 -eq $mockDatabasePermissionInstance2 | Should -BeFalse } } - It 'Should return $false' { - # BUG: Equals only compares the Permission property not State. - # $mockDatabasePermissionInstance1 -eq $mockDatabasePermissionInstance2 | Should -BeFalse - $mockDatabasePermissionInstance1 -eq $mockDatabasePermissionInstance2 | Should -BeTrue + Context 'When comparing Grant with GrantWithGrant' { + It 'Should instantiate two objects' { + $script:mockDatabasePermissionInstance1 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $databasePermissionInstance = [DatabasePermission]::new() + + $databasePermissionInstance.State = 'Grant' + $databasePermissionInstance.Permission = 'select' + + return $databasePermissionInstance + } + + $script:mockDatabasePermissionInstance2 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $databasePermissionInstance = [DatabasePermission]::new() + + $databasePermissionInstance.State = 'GrantWithGrant' + $databasePermissionInstance.Permission = 'select' + + return $databasePermissionInstance + } + } + + It 'Should return $false' { + $mockDatabasePermissionInstance1 -eq $mockDatabasePermissionInstance2 | Should -BeFalse + } } } @@ -192,6 +222,62 @@ Describe 'DatabasePermission' -Tag 'DatabasePermission' { } } + Context 'When calling method GetHashCode()' { + Context 'When two objects are equal' { + It 'Should return the same hash code' { + InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $databasePermissionInstance1 = [DatabasePermission]::new() + $databasePermissionInstance1.State = 'Grant' + $databasePermissionInstance1.Permission = @('Select', 'Update') + + $databasePermissionInstance2 = [DatabasePermission]::new() + $databasePermissionInstance2.State = 'Grant' + $databasePermissionInstance2.Permission = @('Update', 'Select') + + $databasePermissionInstance1.GetHashCode() | Should -Be $databasePermissionInstance2.GetHashCode() + } + } + } + + Context 'When two objects have different State' { + It 'Should return different hash codes' { + InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $databasePermissionInstance1 = [DatabasePermission]::new() + $databasePermissionInstance1.State = 'Grant' + $databasePermissionInstance1.Permission = 'Select' + + $databasePermissionInstance2 = [DatabasePermission]::new() + $databasePermissionInstance2.State = 'Deny' + $databasePermissionInstance2.Permission = 'Select' + + $databasePermissionInstance1.GetHashCode() | Should -Not -Be $databasePermissionInstance2.GetHashCode() + } + } + } + + Context 'When two objects have different Permission' { + It 'Should return different hash codes' { + InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $databasePermissionInstance1 = [DatabasePermission]::new() + $databasePermissionInstance1.State = 'Grant' + $databasePermissionInstance1.Permission = 'Select' + + $databasePermissionInstance2 = [DatabasePermission]::new() + $databasePermissionInstance2.State = 'Grant' + $databasePermissionInstance2.Permission = 'Update' + + $databasePermissionInstance1.GetHashCode() | Should -Not -Be $databasePermissionInstance2.GetHashCode() + } + } + } + } + Context 'When comparing two objects using method CompareTo()' { Context 'When the instance is compared against an invalid object' { It 'Should return a value less than zero' { diff --git a/tests/Unit/Classes/ServerPermission.Tests.ps1 b/tests/Unit/Classes/ServerPermission.Tests.ps1 index b4185d517e..e4dc4a5426 100644 --- a/tests/Unit/Classes/ServerPermission.Tests.ps1 +++ b/tests/Unit/Classes/ServerPermission.Tests.ps1 @@ -71,12 +71,12 @@ Describe 'ServerPermission' -Tag 'ServerPermission' { $script:mockServerPermissionInstance = InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance = [ServerPermission]::new() + $serverPermissionInstance = [ServerPermission]::new() - $databasePermissionInstance.State = 'Grant' - $databasePermissionInstance.Permission = 'ViewServerState' + $serverPermissionInstance.State = 'Grant' + $serverPermissionInstance.Permission = 'ViewServerState' - return $databasePermissionInstance + return $serverPermissionInstance } } @@ -93,17 +93,17 @@ Describe 'ServerPermission' -Tag 'ServerPermission' { InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance1 = [ServerPermission]::new() + $serverPermissionInstance1 = [ServerPermission]::new() - $databasePermissionInstance1.State = 'Grant' - $databasePermissionInstance1.Permission = 'ViewServerState' + $serverPermissionInstance1.State = 'Grant' + $serverPermissionInstance1.Permission = 'ViewServerState' - $databasePermissionInstance2 = [ServerPermission]::new() + $serverPermissionInstance2 = [ServerPermission]::new() - $databasePermissionInstance2.State = 'Grant' - $databasePermissionInstance2.Permission = 'ViewServerState' + $serverPermissionInstance2.State = 'Grant' + $serverPermissionInstance2.Permission = 'ViewServerState' - $databasePermissionInstance1 -eq $databasePermissionInstance2 | Should -BeTrue + $serverPermissionInstance1 -eq $serverPermissionInstance2 | Should -BeTrue } } } @@ -113,81 +113,167 @@ Describe 'ServerPermission' -Tag 'ServerPermission' { InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance1 = [ServerPermission]::new() + $serverPermissionInstance1 = [ServerPermission]::new() - $databasePermissionInstance1.State = 'Grant' - $databasePermissionInstance1.Permission = @('ViewServerState', 'AlterAnyAvailabilityGroup') + $serverPermissionInstance1.State = 'Grant' + $serverPermissionInstance1.Permission = @('ViewServerState', 'AlterAnyAvailabilityGroup') - $databasePermissionInstance2 = [ServerPermission]::new() + $serverPermissionInstance2 = [ServerPermission]::new() - $databasePermissionInstance2.State = 'Grant' - $databasePermissionInstance2.Permission = @('ViewServerState', 'AlterAnyAvailabilityGroup') + $serverPermissionInstance2.State = 'Grant' + $serverPermissionInstance2.Permission = @('ViewServerState', 'AlterAnyAvailabilityGroup') - $databasePermissionInstance1 -eq $databasePermissionInstance2 | Should -BeTrue + $serverPermissionInstance1 -eq $serverPermissionInstance2 | Should -BeTrue } } } } Context 'When object has different value for property State' { + Context 'When comparing Deny with Grant' { + It 'Should instantiate two objects' { + $script:mockServerPermissionInstance1 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $serverPermissionInstance = [ServerPermission]::new() + + $serverPermissionInstance.State = 'Deny' + $serverPermissionInstance.Permission = 'ViewServerState' + + return $serverPermissionInstance + } + + $script:mockServerPermissionInstance2 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $serverPermissionInstance = [ServerPermission]::new() + + $serverPermissionInstance.State = 'Grant' + $serverPermissionInstance.Permission = 'ViewServerState' + + return $serverPermissionInstance + } + } + + It 'Should return $false' { + $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeFalse + } + } + + Context 'When comparing Grant with GrantWithGrant' { + It 'Should instantiate two objects' { + $script:mockServerPermissionInstance1 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $serverPermissionInstance = [ServerPermission]::new() + + $serverPermissionInstance.State = 'Grant' + $serverPermissionInstance.Permission = 'ViewServerState' + + return $serverPermissionInstance + } + + $script:mockServerPermissionInstance2 = InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $serverPermissionInstance = [ServerPermission]::new() + + $serverPermissionInstance.State = 'GrantWithGrant' + $serverPermissionInstance.Permission = 'ViewServerState' + + return $serverPermissionInstance + } + } + + It 'Should return $false' { + $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeFalse + } + } + } + + Context 'When object has different value for property Permission' { It 'Should instantiate two objects' { $script:mockServerPermissionInstance1 = InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance = [ServerPermission]::new() + $serverPermissionInstance = [ServerPermission]::new() - $databasePermissionInstance.State = 'Deny' - $databasePermissionInstance.Permission = 'ViewServerState' + $serverPermissionInstance.State = 'Grant' + $serverPermissionInstance.Permission = 'ViewServerState' - return $databasePermissionInstance + return $serverPermissionInstance } $script:mockServerPermissionInstance2 = InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance = [ServerPermission]::new() + $serverPermissionInstance = [ServerPermission]::new() - $databasePermissionInstance.State = 'Grant' - $databasePermissionInstance.Permission = 'ViewServerState' + $serverPermissionInstance.State = 'Grant' + $serverPermissionInstance.Permission = 'AlterAnyAvailabilityGroup' - return $databasePermissionInstance + return $serverPermissionInstance } } It 'Should return $false' { - #BUG: Another instance where the compare function is not comparing the State parameter - $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeTrue - # $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeFalse + $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeFalse } } + } - Context 'When object has different value for property Permission' { - It 'Should instantiate two objects' { - $script:mockServerPermissionInstance1 = InModuleScope -ScriptBlock { + Context 'When calling method GetHashCode()' { + Context 'When two objects are equal' { + It 'Should return the same hash code' { + InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance = [ServerPermission]::new() + $serverPermissionInstance1 = [ServerPermission]::new() + $serverPermissionInstance1.State = 'Grant' + $serverPermissionInstance1.Permission = @('ViewServerState', 'AlterAnyAvailabilityGroup') - $databasePermissionInstance.State = 'Grant' - $databasePermissionInstance.Permission = 'ViewServerState' + $serverPermissionInstance2 = [ServerPermission]::new() + $serverPermissionInstance2.State = 'Grant' + $serverPermissionInstance2.Permission = @('AlterAnyAvailabilityGroup', 'ViewServerState') - return $databasePermissionInstance + $serverPermissionInstance1.GetHashCode() | Should -Be $serverPermissionInstance2.GetHashCode() } + } + } - $script:mockServerPermissionInstance2 = InModuleScope -ScriptBlock { + Context 'When two objects have different State' { + It 'Should return different hash codes' { + InModuleScope -ScriptBlock { Set-StrictMode -Version 1.0 - $databasePermissionInstance = [ServerPermission]::new() + $serverPermissionInstance1 = [ServerPermission]::new() + $serverPermissionInstance1.State = 'Grant' + $serverPermissionInstance1.Permission = 'ViewServerState' - $databasePermissionInstance.State = 'Grant' - $databasePermissionInstance.Permission = 'AlterAnyAvailabilityGroup' + $serverPermissionInstance2 = [ServerPermission]::new() + $serverPermissionInstance2.State = 'Deny' + $serverPermissionInstance2.Permission = 'ViewServerState' - return $databasePermissionInstance + $serverPermissionInstance1.GetHashCode() | Should -Not -Be $serverPermissionInstance2.GetHashCode() } } + } - It 'Should return $false' { - $script:mockServerPermissionInstance1 -eq $script:mockServerPermissionInstance2 | Should -BeFalse + Context 'When two objects have different Permission' { + It 'Should return different hash codes' { + InModuleScope -ScriptBlock { + Set-StrictMode -Version 1.0 + + $serverPermissionInstance1 = [ServerPermission]::new() + $serverPermissionInstance1.State = 'Grant' + $serverPermissionInstance1.Permission = 'ViewServerState' + + $serverPermissionInstance2 = [ServerPermission]::new() + $serverPermissionInstance2.State = 'Grant' + $serverPermissionInstance2.Permission = 'AlterAnyAvailabilityGroup' + + $serverPermissionInstance1.GetHashCode() | Should -Not -Be $serverPermissionInstance2.GetHashCode() + } } } }