Connect-DbaInstance - Keep the tab completion cache of instance names an array (#10672)

Author: andreasjordan

private/dynamicparams/sqlinstance.ps1

@@ -5,18 +5,13 @@ if (-not [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"]
 
 # Load user-defined instances from config (set via Add-DbaInstanceList)
 foreach ($instance in (Get-DbatoolsConfigValue -FullName "TabExpansion.KnownInstances" -Fallback @())) {
-    if ($instance -and [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] -notcontains $instance) {
-        [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] += $instance
-    }
+    Add-DbaTeppInstanceName -Name $instance
 }
 
 # Load from environment variable (comma-separated list, e.g. set in PowerShell profile)
 if ($env:DBATOOLS_KNOWN_INSTANCES) {
     foreach ($instance in ($env:DBATOOLS_KNOWN_INSTANCES -split ",")) {
-        $lower = $instance.Trim().ToLowerInvariant()
-        if ($lower -and [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] -notcontains $lower) {
-            [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] += $lower
-        }
+        Add-DbaTeppInstanceName -Name $instance
     }
 }
 #endregion Initialize Cache

private/functions/tabcompletion/Add-DbaTeppInstanceName.ps1

@@ -0,0 +1,39 @@
+function Add-DbaTeppInstanceName {
+    <#
+    .SYNOPSIS
+        Adds instance names to the tab completion cache for -SqlInstance, keeping the cache an array.
+
+    .DESCRIPTION
+        The cache [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] starts as $null.
+        Appending to it with += turned it into a string, after which += concatenated every further name
+        onto that string and -notcontains compared the whole string, so every later connection appended
+        its name again and the completer offered one long blob. This function rebuilds the cache as an
+        array every time, lower cases the names like the completer expects, and skips duplicates.
+
+    .PARAMETER Name
+        The instance names to add. Empty entries are ignored.
+
+    .EXAMPLE
+        PS C:\> Add-DbaTeppInstanceName -Name $instance.FullSmoName
+
+        Adds the instance to the completion cache of the current session.
+    #>
+    [CmdletBinding()]
+    param (
+        [AllowEmptyString()]
+        [AllowNull()]
+        [string[]]$Name
+    )
+
+    $knownInstances = @([Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] | Where-Object { $PSItem })
+    foreach ($item in $Name) {
+        if (-not $item) {
+            continue
+        }
+        $lower = $item.Trim().ToLowerInvariant()
+        if ($lower -and $knownInstances -notcontains $lower) {
+            $knownInstances += $lower
+        }
+    }
+    [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] = $knownInstances
+}

public/Add-DbaInstanceList.ps1

@@ -84,9 +84,7 @@ function Add-DbaInstanceList {
             }
 
             # Update the TEPP cache immediately for this session
-            if ([Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] -notcontains $lower) {
-                [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] += $lower
-            }
+            Add-DbaTeppInstanceName -Name $lower
         }
     }
 

public/Connect-DbaInstance.ps1

@@ -1534,9 +1534,7 @@ SELECT SERVERPROPERTY('ProductVersion')
                     [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::SetInstance($instance.FullSmoName.ToLowerInvariant(), $teppConnectionContext, ($server.ConnectionContext.FixedServerRoles -match "SysAdmin"))
 
                     # Update cache for instance names
-                    if ([Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] -notcontains $instance.FullSmoName.ToLowerInvariant()) {
-                        [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"] += $instance.FullSmoName.ToLowerInvariant()
-                    }
+                    Add-DbaTeppInstanceName -Name $instance.FullSmoName
 
                     # Update lots of registered stuff
                     # Default for [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::TeppSyncDisabled is $true, so will not run by default

tests/Add-DbaInstanceList.Tests.ps1

@@ -23,10 +23,11 @@ Describe $CommandName -Tag UnitTests {
 Describe $CommandName -Tag IntegrationTests {
     BeforeAll {
         $instanceName = "dbatoolsci_testinstance_$(Get-Random)"
+        $secondInstanceName = "dbatoolsci_testinstance2_$(Get-Random)"
     }
 
     AfterAll {
-        $null = Remove-DbaInstanceList -SqlInstance $instanceName -ErrorAction SilentlyContinue
+        $null = Remove-DbaInstanceList -SqlInstance $instanceName, $secondInstanceName -ErrorAction SilentlyContinue
     }
 
     Context "adds instances to the list" {
@@ -49,5 +50,17 @@ Describe $CommandName -Tag IntegrationTests {
             $result = Get-DbaInstanceList
             ($result | Where-Object { $PSItem -eq $instanceName.ToLowerInvariant() }).Count | Should -Be 1
         }
+
+        It "keeps the TEPP cache an array when more than one instance is known" {
+            # The cache starts as $null, and $null += "x" made it a string. Every further name was then
+            # concatenated onto that string and the completer offered one long blob instead of names.
+            Add-DbaInstanceList -SqlInstance $secondInstanceName
+            Add-DbaInstanceList -SqlInstance $instanceName
+            $cache = [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"]
+            $cache -is [array] | Should -BeTrue
+            $cache | Should -Contain $instanceName.ToLowerInvariant()
+            $cache | Should -Contain $secondInstanceName.ToLowerInvariant()
+            @($cache | Where-Object { $PSItem -eq $instanceName.ToLowerInvariant() }).Count | Should -Be 1
+        }
     }
 }

tests/Connect-DbaInstance.Tests.ps1

@@ -1054,4 +1054,17 @@ SELECT SERVERPROPERTY('ProductVersion')
             $tsqlVersionMajor | Should -Be $smoVersionMajor
         }
     }
+
+    Context "tab completion cache of instance names" {
+        It "keeps every connected instance as its own entry" {
+            # The cache starts as $null, and $null += "x" made it a string. Every further name was then
+            # concatenated onto that string and the completer offered one long blob instead of names.
+            $null = Connect-DbaInstance -SqlInstance $TestConfig.InstanceMulti1
+            $null = Connect-DbaInstance -SqlInstance $TestConfig.InstanceMulti2
+            $cache = [Dataplat.Dbatools.TabExpansion.TabExpansionHost]::Cache["sqlinstance"]
+            $cache -is [array] | Should -BeTrue
+            $cache | Should -Contain $TestConfig.InstanceMulti1.ToLowerInvariant()
+            $cache | Should -Contain $TestConfig.InstanceMulti2.ToLowerInvariant()
+        }
+    }
 }