Fixes from PR#89 (and initially from PR#59) (also fixes #58)

[johlju]

This is the new PR (again) from the PR #89 (and initially from PR #59)

This is from the work by @Matticusau in the closed PR #59 and closed PR #23, which @aultt continued the work on in PR #89. Now @johlju is continuing the work to get this merged.

---

This change is

[msftclas]

Hi @johlju, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[johlju]

Wait for review on this one, I will go thru all the comments in #89 and fix those first. I let you know when it's ready for review. Just wanted to see that I got all files in.

[johlju]

This one is ready for review now. Resolved all the comments from the review of PR #89.

[johlju]

Maintainers have a lot of work, so until they come around maybe someone in the community could start the review on this?

[johlju]

@mbreakey3 When you have time, could you review this PR?

[johlju]

I squashed (concatenated) a lot of commits.

[mbreakey3]

Reviewed 5 of 8 files at r3.
Review status: 5 of 8 files reviewed at latest revision, 23 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 28 at r3 (raw file):

        Protocol = [System.String]
        ServerName = [System.String]
        TCPPort = [System.Int32]

TcpPort

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 35 at r3 (raw file):

    if ($null -ne (Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name $Name -ErrorAction SilentlyContinue))
    {
        $itemValue = Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\MSSQLServer\Client\ConnectTo' -Name $Name -ErrorAction SilentlyContinue

You shouldn't need to call this twice - just put this line above the if statement and then check that ($null -ne $itemValue)

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 39 at r3 (raw file):

        $returnValue.Name = $Name
        $itemConfig = $itemValue."$Name" -split ','
        if ($itemConfig[0] -eq 'DBMSSOCN')

$itemConfig needs to be checked that it's non-null before indexing into it

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 42 at r3 (raw file):

        {
            $returnValue.Protocol = 'TCP'
            $returnValue.ServerName = $itemConfig[1]

before indexing into the area - $itemConfig needs to be checked to ensure it has at least that many items

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 76 at r3 (raw file):

        [System.Int32]
        $TCPPort,

TcpPort

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 105 at r3 (raw file):

        if ($null -ne $currentValue -and $itemValue -ne $currentValue)
        {
            if ($PSCmdlet.ShouldProcess($Name,"Changing the client alias (64-bit)"))

single quotes around 'Changing...'

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 113 at r3 (raw file):

        elseif ($null -eq $currentValue)
        {
            if ($PSCmdlet.ShouldProcess($Name,"Create client alias (64-bit)"))

single quotes around 'Create...'

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 135 at r3 (raw file):

            if ($null -ne $currentValue -and $itemValue -ne $currentValue)
            {
                if ($PSCmdlet.ShouldProcess($Name,"Changing the client alias (32-bit)"))

all of these should be single quotes

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r3 (raw file):

                    Write-Debug -Message 'Named Pipes'

                    if ($itemConfig[0] -ne 'DBNMPNTW')

again, check to make sure this isn't null first

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 239 at r3 (raw file):

                    }

                    if ($itemConfig[1] -ne "\\$ServerName\PIPE\sql\query")

check this as well to ensure there's >1 item in the area before indexing into it

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 248 at r3 (raw file):

                {
                    Write-Debug -Message 'TCP'
                    if ($itemConfig[0] -ne 'DBMSSOCN')

check for not null

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 253 at r3 (raw file):

                    }

                    if ($itemConfig[1] -ne $ServerName)

check number of items in array

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 258 at r3 (raw file):

                    }

                    if ($itemConfig[2] -ne $TCPPort)

check number of items in array

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 279 at r3 (raw file):

                            Write-Debug -Message 'Named Pipes'

                            if ($itemConfig[0] -ne 'DBNMPNTW')

see above comment

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 294 at r3 (raw file):

                            Write-Debug -Message 'TCP'

                            if ($itemConfig[0] -ne 'DBMSSOCN')

see above comment

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 25 at r3 (raw file):

try
{
    #region Pester Test Initialization

delete these two lines since you don't have anything here

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 43 at r3 (raw file):

            $SqlAlias.Protocol = 'TCP'    
        }
    }

Could you add another test for Get? One for NP probably

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 99 at r3 (raw file):

        }

        It 'Should call any Remove-ItemProperty exactly 2 times (1 for 32bit and 1 for 64 bit reg keys) when alias should be absent' {

can you re-word this It?

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 128 at r3 (raw file):

        } -ModuleName $script:DSCResourceName

        It 'Should return $true when Test is passed as Alias thats already set'{

'...is passed an Alias...'

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 132 at r3 (raw file):

        }

        It 'Should return $false when Test is passed as Alias that is not set'{

see above comment

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 106 at r3 (raw file):

        Mock -CommandName Connect-SQL -MockWith {
            # build a custom object to return which is close to the real SMO object
            $smoObj = [PSCustomObject]@{

space between ']' and '@'

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 199 at r3 (raw file):

                    Name = 'Node01'
                }
                , [PSCustomObject]@{

commas should be on the previous line - also space betwen ] and @

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 262 at r3 (raw file):

                'Microsoft.SqlServer.Management.Smo.AvailabilityReplica' {
                    $object = [PSCustomObject]@{

space between ] and @

---

Comments from Reviewable

[johlju]

Reworking the xSqlAlias resource and the pester test. Finished with the resource yesterday, now I'm working on improving the pester test.

[johlju]

Reworked the entire xSqlAlias resource and it's entire test. Fixing the comments in the previous code would have made the validation logic even harder to read. I hope now I got a better base that will be easier to improve on when I get more review comments. 😄
The resource is fully tested and I also tried to make the test hit every aspect. Let's see what the reviewer find :)

@mbreakey3 when you have time, please continue the review.

---

Reviewed 4 of 4 files at r4, 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 24 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 28 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

TcpPort

Done.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 35 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

You shouldn't need to call this twice - just put this line above the if statement and then check that ($null -ne $itemValue)

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 39 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

$itemConfig needs to be checked that it's non-null before indexing into it

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 42 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

before indexing into the area - $itemConfig needs to be checked to ensure it has at least that many items

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 76 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

TcpPort

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 105 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

single quotes around 'Changing...'

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 113 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

single quotes around 'Create...'

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 135 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

all of these should be single quotes

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

again, check to make sure this isn't null first

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 239 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

check this as well to ensure there's >1 item in the area before indexing into it

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 248 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

check for not null

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 253 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

check number of items in array

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 258 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

check number of items in array

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 279 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

see above comment

Reworked the entire resource. Please review from the beginning again.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 294 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

see above comment

Reworked the entire resource. Please review from the beginning again.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 25 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

delete these two lines since you don't have anything here

Reworked the entire test. Please review from the beginning again.

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 43 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

Could you add another test for Get? One for NP probably

Done

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 99 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

can you re-word this It?

It's not there anymore in the reworked test

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 128 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

'...is passed an Alias...'

It's not there anymore in the reworked test

---

Tests/Unit/MSFT_xSqlAlias.Tests.ps1, line 132 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

see above comment

It's not there anymore in the reworked test

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 106 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

space between ']' and '@'

Done

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 199 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

commas should be on the previous line - also space betwen ] and @

Done

---

Tests/Unit/MSFT_xSQLAOGroupEnsure.Tests.ps1, line 262 at r3 (raw file):

Previously, mbreakey3 (Mariah) wrote…

space between ] and @

Done (found a bunch, fixed them all)

---

Comments from Reviewable

[mbreakey3]

Reviewed 3 of 4 files at r4, 1 of 1 files at r5, 2 of 2 files at r6.
Review status: all files reviewed at latest revision, 19 unresolved discussions.

---

README.md, line 333 at r6 (raw file):

### xSqlAlias
 * **Ensure**: Determines whether the alias should be added or removed. Default values is 'Present'

'Default value...'

---

README.md, line 336 at r6 (raw file):

 * **Name**: (Key) The name of Alias (e.g. svr01\inst01).
 * **ServerName**: (Key) The SQL Server you are aliasing (the netbios name or FQDN).
 * **Protocol**: Protocol to use when connecting. Valid values are 'TCP' or 'NP' (Named Pipes). Default values is 'TCP'.

'Default value...'

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 230 at r6 (raw file):

    if ($result) 
    {
        Write-Verbose "In the desired state"

single quotes

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r6 (raw file):

    else
    {
        Write-Verbose "Not in the desired state"

single quotes - also add in Parameter name '-Message' in this one and the one above

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 5 at r6 (raw file):

{
    [Key, Description("The name of Alias (e.g. svr01\\inst01).")] String Name;
    [Write, Description("Protocol to use when connecting. Valid values are 'TCP' or 'NP' (Named Pipes). Default values is 'TCP'."), ValueMap{"TCP","NP"}, Values{"TCP","NP"}] String Protocol;

Default 'value'

---

Comments from Reviewable

[johlju]

Reviewed 3 of 3 files at r7.
Review status: all files reviewed at latest revision, 19 unresolved discussions.

---

README.md, line 333 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Default value...'

Done. Also changed xSqlAlias to xSQLAlias here in the header and in the unreleased section of the README.md.

---

README.md, line 336 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

'Default value...'

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 230 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

single quotes

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

single quotes - also add in Parameter name '-Message' in this one and the one above

Done

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.schema.mof, line 5 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

Default 'value'

Done. Also found one in the Ensure parameter.

---

Comments from Reviewable

[mbreakey3]

Review status: all files reviewed at latest revision, 15 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r6 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Done

Still need to add -Message parameter

---

Comments from Reviewable

[johlju]

Review status: 7 of 8 files reviewed at latest revision, 15 unresolved discussions.

---

DSCResources/MSFT_xSQLAlias/MSFT_xSQLAlias.psm1, line 234 at r6 (raw file):

Previously, mbreakey3 (Mariah) wrote…

Still need to add -Message parameter

Ahh, my bad. I read that before changing the code but forgot to change it. And in reviewable I just checked the code so it was okay and typed 'Done' without reading your comment again. Will not happen again. One learns by ones mistake 😄

Really done now! :)

---

Comments from Reviewable

[mbreakey3]

---

Reviewed 2 of 3 files at r7, 1 of 1 files at r8.
Review status: all files reviewed at latest revision, 15 unresolved discussions.

---

Comments from Reviewable