ActiveDirectoryDsc.Common: Refactor Remove-DuplicateMembers (#471)

- Changes to ActiveDirectoryDsc.Common: - Refactor `Remove-DuplicateMembers` and added more unit tests (issue #443). - Minor cleanup in `Test-Members` because of the improved `Remove-DuplicateMembers`. - Minor cleanup in `Assert-MemberParameters` because of the improved `Remove-DuplicateMembers`.
dsccommunity · Aug 6, 2019 · 1960969 · 1960969
1 parent 7af106c
commit 1960969
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,7 +3,8 @@
 ## Unreleased
 
 - Changes to ActiveDirectoryDsc
-  - BREAKING CHANGE: ADRecycleBin is replaced by the new resource ADOptionalFeature ([issue #162](https://github.com/PowerShell/ActiveDirectoryDsc/issues/162)).
+  - BREAKING CHANGE: ADRecycleBin is replaced by the new resource ADOptionalFeature
+    ([issue #162](https://github.com/PowerShell/ActiveDirectoryDsc/issues/162)).
   - New resource ADOptionalFeature ([issue #162](https://github.com/PowerShell/ActiveDirectoryDsc/issues/162)).
   - BREAKING CHANGE: Renamed the xActiveDirectory to ActiveDirectoryDsc
     and removed the 'x' from all resource names ([issue #312](https://github.com/PowerShell/ActiveDirectoryDsc/issues/312)).
@@ -42,9 +43,12 @@
     - Add-TypeAssembly
     - New-ADDirectoryContext
   - Changes to ActiveDirectoryDsc.Common:
-    - Removed unused parameter `ModuleName` from `Assert-MemberParameters` function.
-    - Removed unused parameter `ModuleName` from `ConvertTo-DeploymentForestMode` function.
-    - Removed unused parameter `ModuleName` from `ConvertTo-DeploymentDomainMode` function.
+    - Removed unused parameter `ModuleName` from `Assert-MemberParameters`
+      function.
+    - Removed unused parameter `ModuleName` from `ConvertTo-DeploymentForestMode`
+      function.
+    - Removed unused parameter `ModuleName` from `ConvertTo-DeploymentDomainMode`
+      function.
     - Added function help ([issue #321](https://github.com/PowerShell/ActiveDirectoryDsc/issues/321)).
     - Removed the helper function `ThrowInvalidOperationError` and
       `ThrowInvalidArgumentError` in favor of the
@@ -57,6 +61,10 @@
       `Credential` in the function `Get-ADCommonParameters`
     - Added function `Find-DomainController`.
     - Added function `Get-CurrentUser` (moved from the resource ADKDSKey).
+    - Refactor `Remove-DuplicateMembers` and added more unit tests
+      ([issue #443](https://github.com/PowerShell/ActiveDirectoryDsc/issues/443)).
+    - Minor cleanup in `Test-Members` because of the improved `Remove-DuplicateMembers`.
+    - Minor cleanup in `Assert-MemberParameters` because of the improved `Remove-DuplicateMembers`.
   - Updated all the examples files to be prefixed with the resource
     name so they are more easily discovered in PowerShell Gallery and
     Azure Automation ([issue #416](https://github.com/PowerShell/ActiveDirectoryDsc/issues/416)).

diff --git a/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 b/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1
@@ -697,12 +697,12 @@ function Assert-MemberParameters
 
     if ($PSBoundParameters.ContainsKey('MembersToInclude'))
     {
-        $MembersToInclude = [System.String[]] @(Remove-DuplicateMembers -Members $MembersToInclude)
+        $MembersToInclude = Remove-DuplicateMembers -Members $MembersToInclude
     }
 
     if ($PSBoundParameters.ContainsKey('MembersToExclude'))
     {
-        $MembersToExclude = [System.String[]] @(Remove-DuplicateMembers -Members $MembersToExclude)
+        $MembersToExclude = Remove-DuplicateMembers -Members $MembersToExclude
     }
 
     if (($PSBoundParameters.ContainsKey('MembersToInclude')) -and ($PSBoundParameters.ContainsKey('MembersToExclude')))
@@ -728,10 +728,14 @@ function Assert-MemberParameters
 
 <#
     .SYNOPSIS
-        Remove duplicate case insensitive strings (members) from a string array.
+        Remove duplicate members from a string array. The comparison is
+        case insensitive.
 
     .PARAMETER Members
         The array of members to remove duplicates from.
+
+    .OUTPUTS
+        A string array with the unique members-
 #>
 function Remove-DuplicateMembers
 {
@@ -744,38 +748,20 @@ function Remove-DuplicateMembers
         $Members
     )
 
-    Set-StrictMode -Version Latest
-
-    $destIndex = 0
-
-    for ($sourceIndex = 0 ; $sourceIndex -lt $Members.Count; $sourceIndex++)
+    if ($null -eq $Members -or $Members.Count -eq 0)
     {
-        $matchFound = $false
-
-        for ($matchIndex = 0; $matchIndex -lt $destIndex; $matchIndex++)
-        {
-            if ($Members[$sourceIndex] -eq $Members[$matchIndex])
-            {
-                # A duplicate is found. Discard the duplicate.
-                Write-Verbose -Message ($script:localizedData.RemovingDuplicateMember -f $Members[$sourceIndex])
-                $matchFound = $true
-                continue
-            }
-        }
-
-        if (!$matchFound)
-        {
-            $Members[$destIndex++] = $Members[$sourceIndex].ToLowerInvariant()
-        }
+        $uniqueMembers = [System.String[]] @()
+    }
+    else
+    {
+        $uniqueMembers = [System.String[]] ($members | Sort-Object -Unique)
     }
 
-    # Create the output array.
-    $destination = New-Object -TypeName 'System.String[]' -ArgumentList $destIndex
-
-    # Copy only distinct elements from the original array to the destination array.
-    [System.Array]::Copy($Members, $destination, $destIndex)
-
-    return $destination
+    <#
+        Comma make sure we return the string array as the correct type,
+        and also make sure one entry is returned as a string array.
+    #>
+    return ,$uniqueMembers
 } #end function RemoveDuplicateMembers
 
 <#
@@ -831,7 +817,7 @@ function Test-Members
 
         Write-Verbose ($script:localizedData.CheckingMembers -f 'Explicit')
 
-        $Members = [System.String[]] @(Remove-DuplicateMembers -Members $Members)
+        $Members = Remove-DuplicateMembers -Members $Members
 
         if ($ExistingMembers.Count -ne $Members.Count)
         {
@@ -866,7 +852,7 @@ function Test-Members
 
         Write-Verbose -Message ($script:localizedData.CheckingMembers -f 'Included')
 
-        $MembersToInclude = [System.String[]] @(Remove-DuplicateMembers -Members $MembersToInclude)
+        $MembersToInclude = Remove-DuplicateMembers -Members $MembersToInclude
 
         $isInDesiredState = $true
 
@@ -895,7 +881,7 @@ function Test-Members
 
         Write-Verbose -Message ($script:localizedData.CheckingMembers -f 'Excluded')
 
-        $MembersToExclude = [System.String[]] @(Remove-DuplicateMembers -Members $MembersToExclude)
+        $MembersToExclude = Remove-DuplicateMembers -Members $MembersToExclude
 
         $isInDesiredState = $true
 

diff --git a/Modules/ActiveDirectoryDsc.Common/en-US/ActiveDirectoryDsc.Common.strings.psd1 b/Modules/ActiveDirectoryDsc.Common/en-US/ActiveDirectoryDsc.Common.strings.psd1
@@ -20,7 +20,6 @@ ConvertFrom-StringData @'
     CheckingMembers                     = Checking for '{0}' members. (ADCOMMON0019)
     MembershipCountMismatch             = Membership count is not correct. Expected '{0}' members, actual '{1}' members. (ADCOMMON0020)
     MemberNotInDesiredState             = Member '{0}' is not in the desired state. (ADCOMMON0021)
-    RemovingDuplicateMember             = Removing duplicate member '{0}' definition. (ADCOMMON0022)
     MembershipInDesiredState            = Membership is in the desired state. (ADCOMMON0023)
     MembershipNotDesiredState           = Membership is NOT in the desired state. (ADCOMMON0024)
     CheckingSite                        = Checking for site '{0}'. (ADCOMMON0026)

diff --git a/Tests/Unit/ActiveDirectory.Common.Tests.ps1 b/Tests/Unit/ActiveDirectory.Common.Tests.ps1
@@ -695,29 +695,54 @@ InModuleScope 'ActiveDirectoryDsc.Common' {
     }
 
     Describe 'ActiveDirectoryDsc.Common\Remove-DuplicateMembers' {
-        It 'Removes one duplicate' {
+        It 'Should removes one duplicate' {
             $members = Remove-DuplicateMembers -Members 'User1','User2','USER1'
+            $members -is [System.String[]] | Should -BeTrue
 
             $members.Count | Should -Be 2
             $members -contains 'User1' | Should -Be $true
             $members -contains 'User2' | Should -Be $true
         }
 
-        It 'Removes two duplicates' {
+        It 'Should removes two duplicates' {
             $members = Remove-DuplicateMembers -Members 'User1','User2','USER1','USER2'
+            $members -is [System.String[]] | Should -BeTrue
 
             $members.Count | Should -Be 2
             $members -contains 'User1' | Should -Be $true
             $members -contains 'User2' | Should -Be $true
         }
 
-        It 'Removes double duplicates' {
+        It 'Should removes double duplicates' {
             $members = Remove-DuplicateMembers -Members 'User1','User2','USER1','user1'
+            $members -is [System.String[]] | Should -BeTrue
 
             $members.Count | Should -Be 2
             $members -contains 'User1' | Should -Be $true
             $members -contains 'User2' | Should -Be $true
         }
+
+        It 'Should return a string array with one one entry' {
+            $members = Remove-DuplicateMembers -Members 'User1','USER1','user1'
+            $members -is [System.String[]] | Should -BeTrue
+
+            $members.Count | Should -Be 1
+            $members -contains 'User1' | Should -Be $true
+        }
+
+        It 'Should return empty collection when passed a $null value' {
+            $members = Remove-DuplicateMembers -Members $null
+            $members -is [System.String[]] | Should -BeTrue
+
+            $members.Count | Should -Be 0
+        }
+
+        It 'Should return empty collection when passed an empty collection' {
+            $members = Remove-DuplicateMembers -Members @()
+            $members -is [System.String[]] | Should -BeTrue
+
+            $members.Count | Should -Be 0
+        }
     }
 
     Describe 'ActiveDirectoryDsc.Common\Test-Members' {