Changes to SqlServerConfiguration

- Fixed minor typos in comment-based help. - Now the verbose message say what option is changing and to what value (issue dsccommunity#1014). - Change the type of the parameter from SInt32 to UInt32. - Added localization (issue dsccommunity#605).
johlju · Jan 16, 2018 · fe11e04 · fe11e04
1 parent 4678b02
commit fe11e04
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -86,6 +86,10 @@
 - Changes to SqlServerConfiguration
   - Fixed minor typos in comment-based help.
   - Style cleanup in tests.
+  - Now the verbose message say what option is changing and to what value
+    ([issue #1014](https://github.com/PowerShell/SqlServerDsc/issues/1014)).
+  - Change the type of the parameter from SInt32 to UInt32.
+  - Added localization ([issue #605](https://github.com/PowerShell/SqlServerDsc/issues/605)).
 - Changes to SqlServerEndpoint
   - Updated README.md with links to the examples
     ([issue #504](https://github.com/PowerShell/SqlServerDsc/issues/504)).

diff --git a/DSCResources/MSFT_SqlServerConfiguration/MSFT_SqlServerConfiguration.psm1 b/DSCResources/MSFT_SqlServerConfiguration/MSFT_SqlServerConfiguration.psm1
@@ -1,7 +1,12 @@
-# Load Common Code
 Import-Module -Name (Join-Path -Path (Split-Path (Split-Path $PSScriptRoot -Parent) -Parent) `
         -ChildPath 'SqlServerDscHelper.psm1') `
     -Force
+
+Import-Module -Name (Join-Path -Path (Split-Path -Path $PSScriptRoot -Parent) `
+    -ChildPath 'CommonResourceHelper.psm1')
+
+$script:localizedData = Get-LocalizedData -ResourceName 'MSFT_SqlServerConfiguration'
+
 <#
     .SYNOPSIS
     Gets the current value of a SQL configuration option
@@ -20,39 +25,41 @@ Import-Module -Name (Join-Path -Path (Split-Path (Split-Path $PSScriptRoot -Pare
 
     .PARAMETER RestartService
     *** Not used in this function ***
-    Determines whether the instance should be restarted after updating the configuration option.
+    Determines whether the instance should be restarted after updating the
+    configuration option.
 
     .PARAMETER RestartTimeout
     *** Not used in this function ***
-    The length of time, in seconds, to wait for the service to restart. Default is 120 seconds.
+    The length of time, in seconds, to wait for the service to restart. Default
+    is 120 seconds.
 #>
 function Get-TargetResource
 {
     [CmdletBinding()]
     [OutputType([System.Collections.Hashtable])]
     param(
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $ServerName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $InstanceName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $OptionName,
 
         [Parameter(Mandatory = $true)]
-        [Int32]
+        [System.Int32]
         $OptionValue,
 
         [Parameter()]
-        [Boolean]
+        [System.Boolean]
         $RestartService = $false,
 
         [Parameter()]
-        [Int32]
+        [System.UInt32]
         $RestartTimeout = 120
     )
 
@@ -61,11 +68,17 @@ function Get-TargetResource
     ## get the configuration option
     $option = $sql.Configuration.Properties | Where-Object { $_.DisplayName -eq $OptionName }
 
-    if (!$option)
+    if (-not $option)
     {
-        throw New-TerminatingError -ErrorType "ConfigurationOptionNotFound" -FormatArgs $OptionName -ErrorCategory InvalidArgument
+        $errorMessage = $script:localizedData.ConfigurationOptionNotFound -f $OptionName
+        New-InvalidArgumentException -ArgumentName 'OptionName' -Message $errorMessage
     }
 
+    Write-Verbose -Message (
+        $script:localizedData.CurrentOptionValue `
+            -f $OptionName, $option.ConfigValue
+    )
+
     return @{
         ServerName     = $ServerName
         InstanceName   = $InstanceName
@@ -93,37 +106,39 @@ function Get-TargetResource
     The desired value of the SQL configuration option
 
     .PARAMETER RestartService
-    Determines whether the instance should be restarted after updating the configuration option
+    Determines whether the instance should be restarted after updating the
+    configuration option
 
     .PARAMETER RestartTimeout
-    The length of time, in seconds, to wait for the service to restart. Default is 120 seconds.
+    The length of time, in seconds, to wait for the service to restart. Default
+    is 120 seconds.
 #>
 function Set-TargetResource
 {
     [CmdletBinding()]
     param(
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $ServerName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $InstanceName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $OptionName,
 
         [Parameter(Mandatory = $true)]
-        [Int32]
+        [System.Int32]
         $OptionValue,
 
         [Parameter()]
-        [Boolean]
+        [System.Boolean]
         $RestartService = $false,
 
         [Parameter()]
-        [Int32]
+        [System.UInt32]
         $RestartTimeout = 120
     )
 
@@ -132,26 +147,37 @@ function Set-TargetResource
     ## get the configuration option
     $option = $sql.Configuration.Properties | Where-Object { $_.DisplayName -eq $OptionName }
 
-    if (!$option)
+    if (-not $option)
     {
-        throw New-TerminatingError -ErrorType "ConfigurationOptionNotFound" -FormatArgs $OptionName -ErrorCategory InvalidArgument
+        $errorMessage = $script:localizedData.ConfigurationOptionNotFound -f $OptionName
+        New-InvalidArgumentException -ArgumentName 'OptionName' -Message $errorMessage
     }
 
     $option.ConfigValue = $OptionValue
     $sql.Configuration.Alter()
 
     if ($option.IsDynamic -eq $true)
     {
-        New-VerboseMessage -Message 'Configuration option has been updated.'
+        Write-Verbose -Message (
+            $script:localizedData.ConfigurationValueUpdated `
+                -f $OptionName, $OptionValue
+        )
     }
     elseif (($option.IsDynamic -eq $false) -and ($RestartService -eq $true))
     {
-        New-VerboseMessage -Message 'Configuration option has been updated, restarting instance...'
+        Write-Verbose -Message (
+            $script:localizedData.AutomaticRestart `
+                -f $ServerName, $InstanceName
+        )
+
         Restart-SqlService -SQLServer $ServerName -SQLInstanceName $InstanceName -Timeout $RestartTimeout
     }
     else
     {
-        New-WarningMessage -WarningType 'ConfigurationRestartRequired' -FormatArgs $OptionName
+        Write-Warning -Message (
+            $script:localizedData.ConfigurationRestartRequired `
+                -f $OptionName, $OptionValue, $ServerName, $InstanceName
+        )
     }
 }
 
@@ -173,47 +199,68 @@ function Set-TargetResource
 
     .PARAMETER RestartService
     *** Not used in this function ***
-    Determines whether the instance should be restarted after updating the configuration option
+    Determines whether the instance should be restarted after updating the
+    configuration option
 
     .PARAMETER RestartTimeout
     *** Not used in this function ***
-    The length of time, in seconds, to wait for the service to restart.
+    The length of time, in seconds, to wait for the service to restart. Default
+    is 120 seconds.
 #>
 function Test-TargetResource
 {
     [CmdletBinding()]
     [OutputType([Boolean])]
     param(
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $ServerName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $InstanceName,
 
         [Parameter(Mandatory = $true)]
-        [String]
+        [System.String]
         $OptionName,
 
         [Parameter(Mandatory = $true)]
-        [Int32]
+        [System.Int32]
         $OptionValue,
 
         [Parameter()]
-        [Boolean]
+        [System.Boolean]
         $RestartService = $false,
 
         [Parameter()]
-        [Int32]
+        [System.UInt32]
         $RestartTimeout = 120
     )
 
     ## Get the current state of the configuration item
-    $state = Get-TargetResource @PSBoundParameters
+    $getTargetResourceResult = Get-TargetResource @PSBoundParameters
+
+    if ($getTargetResourceResult.OptionValue -eq $OptionValue)
+    {
+        Write-Verbose -Message (
+            $script:localizedData.InDesiredState `
+                -f $OptionName
+        )
+
+        $result = $true
+    }
+    else
+    {
+        Write-Verbose -Message (
+            $script:localizedData.NotInDesiredState `
+                -f $OptionName, $OptionValue, $getTargetResourceResult.OptionValue
+        )
+
+        $result = $false
+    }
 
     ## return whether the value matches the desired state
-    return ($state.OptionValue -eq $OptionValue)
+    return $result
 }
 
 Export-ModuleMember -Function *-TargetResource
diff --git a/DSCResources/MSFT_SqlServerConfiguration/MSFT_SqlServerConfiguration.schema.mof b/DSCResources/MSFT_SqlServerConfiguration/MSFT_SqlServerConfiguration.schema.mof
@@ -4,7 +4,7 @@ class MSFT_SqlServerConfiguration : OMI_BaseResource
     [Key, Description("The hostname of the SQL Server to be configured.")] String ServerName;
     [Key, Description("Name of the SQL instance to be configured.")] String InstanceName;
     [Key, Description("The name of the SQL configuration option to be checked.")] String OptionName;
-    [Required, Description("The desired value of the SQL configuration option.")] Sint32 OptionValue;
+    [Required, Description("The desired value of the SQL configuration option.")] SInt32 OptionValue;
     [Write, Description("Determines whether the instance should be restarted after updating the configuration option.")] Boolean RestartService;
-    [Write, Description("The length of time, in seconds, to wait for the service to restart. Default is 120 seconds.")] Sint32 RestartTimeout;
+    [Write, Description("The length of time, in seconds, to wait for the service to restart. Default is 120 seconds.")] UInt32 RestartTimeout;
 };
diff --git a/DSCResources/MSFT_SqlServerConfiguration/en-US/MSFT_SqlServerConfiguration.strings.psd1 b/DSCResources/MSFT_SqlServerConfiguration/en-US/MSFT_SqlServerConfiguration.strings.psd1
@@ -0,0 +1,11 @@
+# Localized resources for SqlServerConfiguration
+
+ConvertFrom-StringData @'
+    CurrentOptionValue = Configuration option '{0}' has a current value of '{1}'.
+    ConfigurationValueUpdated = Configuration option '{0}' has been updated to value '{1}'.
+    AutomaticRestart = A restart of SQL Server is required for the change to take effect. Initiating automatic restart of the SQL Server instance {0}//{1}.
+    ConfigurationOptionNotFound = Specified option '{0}' could not be found.
+    ConfigurationRestartRequired = Configuration option '{0}' has been updated to value '{1}', but a manual restart of SQL Server instance {2}//{3} is required for it to take effect.
+    NotInDesiredState = Configuration option '{0}' is not in desired state. Expected '{1}', but was '{2}'.
+    InDesiredState = Configuration option '{0}' is in desired state.
+'@
diff --git a/en-US/SqlServerDscHelper.strings.psd1 b/en-US/SqlServerDscHelper.strings.psd1
@@ -91,10 +91,6 @@ ConvertFrom-StringData @'
     PermissionGetError = Unexpected result when trying to get permissions for '{0}'.
     ChangingPermissionFailed = Changing permission for principal '{0}' failed.
 
-    # Configuration
-    ConfigurationOptionNotFound = Specified option '{0}' could not be found.
-    ConfigurationRestartRequired = Configuration option '{0}' has been updated, but a manual restart of SQL Server is required for it to take effect.
-
     # AlwaysOnService
     AlterAlwaysOnServiceFailed = Failed to ensure Always On is {0} on the instance '{1}'.
     UnexpectedAlwaysOnStatus = The status of property Server.IsHadrEnabled was neither $true or $false. Status is '{0}'.