Changes to xActiveDirectory

- Removed the helper function `ThrowInvalidOperationError` in favor of new helper functions for localization (issue dsccommunity#316). - Cleaned up some minor style violations in the code.
johlju · Jul 7, 2019 · e8ae658 · e8ae658
1 parent f81e3b7
commit e8ae658
Show file tree

Hide file tree

Showing 9 changed files with 95 additions and 72 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,17 +3,30 @@
 ## Unreleased
 
 - Changes to xActiveDirectory
-  - Added a Requirements section to every DSC resource README with the bullet point stating "Target machine must be running Windows Server 2008 R2 or later" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)).
-  - Added 'about_\<DSCResource\>.help.txt' file to all resources ([issue #404](https://github.com/PowerShell/xActiveDirectory/pull/404)).
+  - Added a Requirements section to every DSC resource README with the
+    bullet point stating "Target machine must be running Windows Server
+    2008 R2 or later" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)).
+  - Added 'about_\<DSCResource\>.help.txt' file to all resources
+    ([issue #404](https://github.com/PowerShell/xActiveDirectory/pull/404)).
+  - Removed the helper function `ThrowInvalidOperationError` in favor of
+    [new helper functions for localization](https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#helper-functions-for-localization)
+    ([issue #316](https://github.com/PowerShell/xActiveDirectory/pull/316)).
+  - Cleaned up some minor style violations in the code.
 - Changes to xADManagedServiceAccount
-  - Added a requirement to README stating "Group Managed Service Accounts need at least one Windows Server 2012 Domain Controller" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)).
+  - Added a requirement to README stating "Group Managed Service Accounts
+    need at least one Windows Server 2012 Domain Controller"
+    ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)).
 - Changes to xADComputer
-  - Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config. ([issue #410](https://github.com/PowerShell/xActiveDirectory/pull/410))
+  - Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config
+    ([issue #410](https://github.com/PowerShell/xActiveDirectory/pull/410)).
 - Changes to xADOrganizationalUnit
-  - Catch exception when the path property specifies a non-existing path ([issue #408](https://github.com/PowerShell/xActiveDirectory/pull/408))
+  - Catch exception when the path property specifies a non-existing path
+    ([issue #408](https://github.com/PowerShell/xActiveDirectory/pull/408))
 - Changes to xADUser
-  - Fixes exception when creating a user with an empty string property ([issue #407](https://github.com/PowerShell/xActiveDirectory/pull/407)).
-  - Fixes exception when updating `CommonName` and `Path` concurrently ([issue #402](https://github.com/PowerShell/xActiveDirectory/pull/402)).
+  - Fixes exception when creating a user with an empty string property
+    ([issue #407](https://github.com/PowerShell/xActiveDirectory/pull/407)).
+  - Fixes exception when updating `CommonName` and `Path` concurrently
+    ([issue #402](https://github.com/PowerShell/xActiveDirectory/pull/402)).
 
 ## 3.0.0.0
 
@@ -57,7 +70,8 @@
       ([issue #374](https://github.com/PowerShell/xActiveDirectory/issues/374)).
   - Removed unused legacy test files from the root of the repository.
   - Updated Example List README with missing resources.
-  - Added missing examples for xADReplicationSubnet, xADServicePrincipalName and xWaitForADDomain. ([issue #395](https://github.com/PowerShell/xActiveDirectory/issues/395)).
+  - Added missing examples for xADReplicationSubnet, xADServicePrincipalName
+    and xWaitForADDomain. ([issue #395](https://github.com/PowerShell/xActiveDirectory/issues/395)).
 - Changes to xADComputer
   - Refactored the resource and the unit tests.
   - BREAKING CHANGE: The `Enabled` property is **DEPRECATED** and is no
@@ -87,7 +101,8 @@
 - Changes to xADOrganizationalUnit
   - Change the description of the property RestoreFromRecycleBin.
   - Code cleanup.
-  - Fix incorrect verbose message when this resource has Ensure set to Absent ([issue #276](https://github.com/PowerShell/xActiveDirectory/issues/276)).
+  - Fix incorrect verbose message when this resource has Ensure set to
+    Absent ([issue #276](https://github.com/PowerShell/xActiveDirectory/issues/276)).
 - Changes to xADUser
   - Change the description of the property RestoreFromRecycleBin.
   - Added ServicePrincipalNames property ([issue #153](https://github.com/PowerShell/xActiveDirectory/issues/153)).
@@ -173,12 +188,14 @@
     and [@kungfu71186](https://github.com/kungfu71186)
   - Removing the Misc Folder, as it is no longer required.
   - Added xADKDSKey resource to create KDS Root Keys for gMSAs. [@kungfu71186](https://github.com/kungfu71186)
-  - Combined DscResource.LocalizationHelper and DscResource.Common Modules into xActiveDirectory.Common
+  - Combined DscResource.LocalizationHelper and DscResource.Common Modules
+    into xActiveDirectory.Common
 - Changes to xADReplicationSiteLink
   - Make use of the new localization helper functions.
 - Changes to xAdDomainController
   - Added new parameter to disable or enable the Global Catalog (GC)
-    ([issue #75](https://github.com/PowerShell/xActiveDirectory/issues/75)). [Eric Foskett @Merto410](https://github.com/Merto410)
+    ([issue #75](https://github.com/PowerShell/xActiveDirectory/issues/75)).
+    [Eric Foskett @Merto410](https://github.com/Merto410)
   - Fixed a bug with the parameter `InstallationMediaPath` that it would
     not be added if it was specified in a configuration. Now the parameter
     `InstallationMediaPath` is correctly passed to `Install-ADDSDomainController`.

diff --git a/DSCResources/MSFT_xADDomain/MSFT_xADDomain.psm1 b/DSCResources/MSFT_xADDomain/MSFT_xADDomain.psm1
@@ -150,7 +150,7 @@ function Get-TargetResource
         catch [Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException]
         {
             $errorMessage = $script:localizedData.ExistingDomainMemberError -f $DomainName
-            ThrowInvalidOperationError -ErrorId 'xADDomain_DomainMember' -ErrorMessage $errorMessage
+            New-ObjectNotFoundException -Message $errorMessage -ErrorRecord $_
         }
         catch [Microsoft.ActiveDirectory.Management.ADServerDownException]
         {
@@ -161,7 +161,7 @@ function Get-TargetResource
         catch [System.Security.Authentication.AuthenticationException]
         {
             $errorMessage = $script:localizedData.InvalidCredentialError -f $DomainName
-            ThrowInvalidOperationError -ErrorId 'xADDomain_InvalidCredential' -ErrorMessage $errorMessage
+            New-InvalidOperationException -Message $errorMessage -ErrorRecord $_
         }
         catch
         {

diff --git a/DSCResources/MSFT_xADGroup/MSFT_xADGroup.psm1 b/DSCResources/MSFT_xADGroup/MSFT_xADGroup.psm1
@@ -103,7 +103,7 @@ function Get-TargetResource
         Write-Verbose -Message ($script:localizedData.RetrievingGroupMembers -f $MembershipAttribute)
 
         # Retrieve the current list of members, returning the specified membership attribute
-        [System.Array]$adGroupMembers = (Get-ADGroupMember @adGroupParams).$MembershipAttribute
+        [System.Array] $adGroupMembers = (Get-ADGroupMember @adGroupParams).$MembershipAttribute
 
         $targetResource = @{
             GroupName           = $adGroup.Name

diff --git a/Modules/xActiveDirectory.Common/en-US/xActiveDirectory.Common.strings.psd1 b/Modules/xActiveDirectory.Common/en-US/xActiveDirectory.Common.strings.psd1
@@ -15,7 +15,7 @@ ConvertFrom-StringData @'
     MembersIsNullError                  = The Members parameter value is null. The '{0}' parameter must be provided if neither '{1}' nor '{2}' is provided. (ADCOMMON0012)
     IncludeAndExcludeConflictError      = The member '{0}' is included in both '{1}' and '{2}' parameter values. The same member must not be included in both '{1}' and '{2}' parameter values. (ADCOMMON0014)
     IncludeAndExcludeAreEmptyError      = The '{0}' and '{1}' parameters are either both null or empty.  At least one member must be specified in one of these parameters. (ADCOMMON0015)
-    RecycleBinRestoreFailed             = Restoring {0} ({1}) from the recycle bin failed. Error message: {2}. (ADCOMMON0017)
+    RecycleBinRestoreFailed             = Failed restoring {0} ({1}) from the recycle bin. (ADCOMMON0017)
     EmptyDomainError                    = No domain name retrieved for group member {0} in group {1}. (ADCOMMON0018)
     CheckingMembers                     = Checking for '{0}' members. (ADCOMMON0019)
     MembershipCountMismatch             = Membership count is not correct. Expected '{0}' members, actual '{1}' members. (ADCOMMON0020)

diff --git a/Modules/xActiveDirectory.Common/xActiveDirectory.Common.psm1 b/Modules/xActiveDirectory.Common/xActiveDirectory.Common.psm1
@@ -488,9 +488,8 @@ function Assert-Module
 
     if (-not (Get-Module -Name $ModuleName -ListAvailable))
     {
-        $errorId = '{0}_ModuleNotFound' -f $ModuleName
         $errorMessage = $script:localizedData.RoleNotFoundError -f $moduleName
-        ThrowInvalidOperationError -ErrorId $errorId -ErrorMessage $errorMessage
+        New-ObjectNotFoundException -Message $errorMessage
     }
 
     if ($ImportModule)
@@ -1039,28 +1038,6 @@ function Get-ADCommonParameters
     return $adConnectionParameters
 } #end function Get-ADCommonParameters
 
-function ThrowInvalidOperationError
-{
-    [CmdletBinding()]
-    param
-    (
-        [Parameter(Mandatory = $true)]
-        [ValidateNotNullOrEmpty()]
-        [System.String]
-        $ErrorId,
-
-        [Parameter(Mandatory = $true)]
-        [ValidateNotNullOrEmpty()]
-        [System.String]
-        $ErrorMessage
-    )
-
-    $exception = New-Object -TypeName 'System.InvalidOperationException' -ArgumentList $ErrorMessage
-    $errorCategory = [System.Management.Automation.ErrorCategory]::InvalidOperation
-    $errorRecord = New-Object -TypeName 'System.Management.Automation.ErrorRecord' -ArgumentList @($exception, $ErrorId, $errorCategory, $null)
-    throw $errorRecord
-}
-
 function ThrowInvalidArgumentError
 {
     [CmdletBinding()]
@@ -1277,7 +1254,8 @@ function Restore-ADCommonObject
         catch [Microsoft.ActiveDirectory.Management.ADException]
         {
             # After Get-TargetResource is through, only one error can occur here: Object parent does not exist
-            ThrowInvalidOperationError -ErrorId "$($Identity)_RecycleBinRestoreFailed" -ErrorMessage ($script:localizedData.RecycleBinRestoreFailed -f $Identity, $ObjectClass, $_.Exception.Message)
+            $errorMessage = $script:localizedData.RecycleBinRestoreFailed -f $Identity, $ObjectClass
+            New-InvalidOperationException -Message $errorMessage -ErrorRecord $_
         }
     }
 
@@ -1835,7 +1813,6 @@ Export-ModuleMember -Function @(
     'ConvertTo-TimeSpan'
     'ConvertFrom-TimeSpan'
     'Get-ADCommonParameters'
-    'ThrowInvalidOperationError'
     'ThrowInvalidArgumentError'
     'Test-ADReplicationSite'
     'ConvertTo-DeploymentForestMode'

diff --git a/Tests/Unit/MSFT_xADDomain.Tests.ps1 b/Tests/Unit/MSFT_xADDomain.Tests.ps1
@@ -79,12 +79,17 @@ try
 
             It 'Calls "Assert-Module" to check "ADDSDeployment" module is installed' {
                 Mock -CommandName Get-ADDomain -MockWith {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest     = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
                 }
-                Mock -CommandName Get-ADForest -MockWith { [psobject]@{ForestMode = $mgmtForestMode} }
+
+                Mock -CommandName Get-ADForest -MockWith {
+                    [PSObject] @{
+                        ForestMode = $mgmtForestMode
+                    }
+                }
 
                 $result = Get-TargetResource @testDefaultParams -DomainName $correctDomainName
 
@@ -93,13 +98,17 @@ try
 
             It 'Returns "System.Collections.Hashtable" object type' {
                 Mock -CommandName Get-ADDomain {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest     = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
                 }
 
-                Mock -CommandName Get-ADForest -MockWith { [psobject]@{ForestMode = $mgmtForestMode} }
+                Mock -CommandName Get-ADForest -MockWith {
+                    [PSObject] @{
+                        ForestMode = $mgmtForestMode
+                    }
+                }
 
                 $result = Get-TargetResource @testDefaultParams -DomainName $correctDomainName
 
@@ -109,7 +118,7 @@ try
             It 'Calls "Get-ADDomain" without credentials if domain member' {
                 Mock -CommandName Test-DomainMember -MockWith { $true; }
                 Mock -CommandName Get-ADDomain -ParameterFilter { $Credential -eq $null } -MockWith {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
@@ -123,12 +132,17 @@ try
             It 'Calls "Get-ADForest" without credentials if domain member' {
                 Mock -CommandName Test-DomainMember -MockWith { $true; }
                 Mock -CommandName Get-ADDomain -ParameterFilter { $Credential -eq $null } -MockWith {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
                 }
-                Mock -CommandName Get-ADForest -ParameterFilter { $Credential -eq $null } -MockWith { [psobject]@{ForestMode = $mgmtForestMode} }
+
+                Mock -CommandName Get-ADForest -ParameterFilter { $Credential -eq $null } -MockWith {
+                    [PSObject] @{
+                        ForestMode = $mgmtForestMode
+                    }
+                }
 
                 $result = Get-TargetResource @testDefaultParams -DomainName $correctDomainName
 
@@ -137,49 +151,59 @@ try
 
             It 'Throws "Invalid credentials" when domain is available but authentication fails' {
                 Mock -CommandName Get-ADDomain -ParameterFilter { $Identity.ToString() -eq $incorrectDomainName } -MockWith {
-                    Write-Error -Exception (New-Object System.Security.Authentication.AuthenticationException)
+                    throw New-Object System.Security.Authentication.AuthenticationException
                 }
 
                 # Match operator is case-sensitive!
-                { Get-TargetResource @testDefaultParams -DomainName $incorrectDomainName } | Should -Throw 'invalid credentials'
+                { Get-TargetResource @testDefaultParams -DomainName $incorrectDomainName } | Should -Throw ($script:localizedData.InvalidCredentialError -f $incorrectDomainName)
             }
 
             It 'Throws "Computer is already a domain member" when is already a domain member' {
                 Mock -CommandName Get-ADDomain -ParameterFilter { $Identity.ToString() -eq $incorrectDomainName } -MockWith {
-                    Write-Error -Exception (New-Object Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException)
+                    throw New-Object Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException
                 }
 
-                { Get-TargetResource @testDefaultParams -DomainName $incorrectDomainName } | Should -Throw 'Computer is already a domain member'
+                { Get-TargetResource @testDefaultParams -DomainName $incorrectDomainName } | Should -Throw ($script:localizedData.ExistingDomainMemberError -f $incorrectDomainName)
             }
 
             It 'Does not throw when domain cannot be located' {
                 Mock -CommandName Get-ADDomain -ParameterFilter { $Identity.ToString() -eq $missingDomainName } -MockWith {
-                    Write-Error -Exception (New-Object Microsoft.ActiveDirectory.Management.ADServerDownException)
+                    throw New-Object Microsoft.ActiveDirectory.Management.ADServerDownException
                 }
 
                 { Get-TargetResource @testDefaultParams -DomainName $missingDomainName } | Should -Not -Throw
             }
 
             It 'Returns the correct domain mode' {
                 Mock -CommandName Get-ADDomain -MockWith {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest     = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
                 }
-                Mock -CommandName Get-ADForest -MockWith { [psobject]@{ForestMode = $mgmtForestMode} }
+
+                Mock -CommandName Get-ADForest -MockWith {
+                    [PSObject] @{
+                        ForestMode = $mgmtForestMode
+                    }
+                }
 
                 (Get-TargetResource @testDefaultParams -DomainName $correctDomainName).DomainMode | Should -Be $domainMode
             }
 
             It 'Returns the correct forest mode' {
                 Mock -CommandName Get-ADDomain -MockWith {
-                    [psobject]@{
+                    [PSObject] @{
                         Forest     = $correctDomainName
                         DomainMode = $mgmtDomainMode
                     }
                 }
-                Mock -CommandName Get-ADForest -MockWith { [psobject]@{ForestMode = $mgmtForestMode} }
+
+                Mock -CommandName Get-ADForest -MockWith {
+                    [PSObject] @{
+                        ForestMode = $mgmtForestMode
+                    }
+                }
 
                 (Get-TargetResource @testDefaultParams -DomainName $correctDomainName).ForestMode | Should -Be $forestMode
             }

diff --git a/Tests/Unit/MSFT_xADKDSKey.Tests.ps1 b/Tests/Unit/MSFT_xADKDSKey.Tests.ps1
@@ -248,8 +248,9 @@ try
         Describe -Name 'MSFT_xADKDSKey\Get-ADRootDomainDN' {
             BeforeAll {
                 Mock -CommandName New-Object -MockWith {
-                    $object = [PSCustomObject]@{}
+                    $object = [PSCustomObject] @{}
                     $object | Add-Member -MemberType ScriptMethod -Name 'Get' -Value { return $mockADDomain }
+
                     return $object
                 }
             }

diff --git a/Tests/Unit/MSFT_xADUser.Tests.ps1 b/Tests/Unit/MSFT_xADUser.Tests.ps1
@@ -718,9 +718,11 @@ try
 
                     $script:mockCounter = 0
 
-                    Mock -CommandName Restore-ADCommonObject -MockWith { return [PSCustomObject]@{
+                    Mock -CommandName Restore-ADCommonObject -MockWith {
+                        return [PSCustomObject] @{
                             ObjectClass = 'user'
-                        } }
+                        }
+                    }
 
                     Set-TargetResource @restoreParam