Connect-DbaInstance - Fix AccessToken NonPooledConnection regression (review of #10260)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: potatoqualitee

docs/trackers/features/commit-bug-review-TRACKER.md

@@ -75,7 +75,7 @@ Find real bugs (logic errors, null refs, incorrect behavior) and fix them. Skip
 | 099624061 | Get-DbaDbRestoreHistory - Add BackupStartDate, StopAt, and LastRestorePoint columns (#10249) | DONE | Fixed LastRestorePoint so StopAt is used whenever specified; added unit regression test. |
 | 4f1e56ce4 | New-DbaDbMailAccount, Set-DbaDbMailAccount - Add Port, SSL, and authentication parameters (#10257) | DONE | Added validation to reject conflicting SMTP authentication modes and incomplete credential pairs; added unit regression tests. |
 | 8218d327e | Restore-DbaDatabase, Invoke-DbaAdvancedRestore - Add ErrorBrokerConversations parameter (#10253) | DONE | Fixed missing ExecuteAs script prefix and added NoRecovery/Standby validation; added regression tests. |
-| 9a4e4bacb | Connect-DbaInstance - Set NonPooledConnection on ServerConnection (#10260) | PENDING | |
+| 9a4e4bacb | Connect-DbaInstance - Set NonPooledConnection on ServerConnection (#10260) | DONE | Fixed AccessToken regression by skipping a redundant NonPooledConnection assignment on SqlConnection-backed contexts; added unit regression test. |
 | a0ab78a66 | Get-DbaCmObject - Apply CimOperationTimeout to all CIM connections (#10252) | PENDING | |
 | fbdb47053 | Import-DbaXESessionTemplate - Add event_file target when TargetFilePath specified (#10250) | PENDING | |
 | 3d6fa113f | Connect-DbaInstance - Trust server certificate for localhost DAC connections (#10263) | PENDING | |

public/Connect-DbaInstance.ps1

@@ -1126,7 +1126,7 @@ function Connect-DbaInstance {
                 }
                 Write-Message -Level Debug -Message "Setting ConnectionContext.StatementTimeout to '$StatementTimeout'"
                 $server.ConnectionContext.StatementTimeout = $StatementTimeout
-                if ($NonPooledConnection) {
+                if ($NonPooledConnection -and -not $server.ConnectionContext.NonPooledConnection) {
                     Write-Message -Level Debug -Message "Setting ConnectionContext.NonPooledConnection to 'True'"
                     $server.ConnectionContext.NonPooledConnection = $true
                 }
@@ -1366,4 +1366,4 @@ function Connect-DbaInstance {
             Write-Message -Level Debug -Message "We are finished with this instance"
         }
     }
-}
+}

tests/Connect-DbaInstance.Tests.ps1

@@ -137,6 +137,66 @@ Describe $CommandName -Tag UnitTests {
             $result.ConnectionString | Should -Match "Initial Catalog=master"
         }
     }
+
+    Context "Access token connection behavior" {
+        BeforeAll {
+            function New-MockAccessTokenServer {
+                $sqlConnectionObject = [PSCustomObject]@{
+                    ConnectionString = "Data Source=sqltoken;Integrated Security=True"
+                }
+                $connectionContext = [PSCustomObject]@{
+                    ConnectionString    = $sqlConnectionObject.ConnectionString
+                    SqlConnectionObject = $sqlConnectionObject
+                    StatementTimeout    = 0
+                }
+
+                Add-Member -InputObject $connectionContext -Name NonPooledConnection -MemberType ScriptProperty -Value {
+                    $true
+                } -SecondValue {
+                    param($value)
+                    $script:nonPooledConnectionSetterCalls++
+                    throw "Property NonPooledConnection cannot be changed or read after a connection string has been set."
+                } -Force
+
+                Add-Member -InputObject $connectionContext -Name ExecuteWithResults -MemberType ScriptMethod -Value {
+                    param($Query)
+                } -Force
+
+                [PSCustomObject]@{
+                    ConnectionContext = $connectionContext
+                }
+            }
+
+            Mock Add-ConnectionHashValue { } -ModuleName dbatools
+            Mock New-Object {
+                [PSCustomObject]@{
+                    ConnectionString = "Data Source=sqltoken;Integrated Security=True"
+                    AccessToken      = $null
+                }
+            } -ModuleName dbatools -ParameterFilter {
+                $TypeName -eq "Microsoft.Data.SqlClient.SqlConnection"
+            }
+            Mock New-Object {
+                [PSCustomObject]@{ }
+            } -ModuleName dbatools -ParameterFilter {
+                $TypeName -eq "Microsoft.SqlServer.Management.Common.ServerConnection"
+            }
+            Mock New-Object {
+                New-MockAccessTokenServer
+            } -ModuleName dbatools -ParameterFilter {
+                $TypeName -eq "Microsoft.SqlServer.Management.Smo.Server"
+            }
+        }
+
+        It "does not reapply NonPooledConnection when AccessToken already uses a SqlConnection" {
+            $script:nonPooledConnectionSetterCalls = 0
+
+            $result = Connect-DbaInstance -SqlInstance "sqltoken" -AccessToken "token" -NonPooledConnection -SqlConnectionOnly
+
+            $result.ConnectionString | Should -Be "Data Source=sqltoken;Integrated Security=True"
+            $script:nonPooledConnectionSetterCalls | Should -Be 0
+        }
+    }
 }
 
 Describe $CommandName -Tag IntegrationTests {
@@ -396,4 +456,4 @@ Describe $CommandName -Tag IntegrationTests {
             $null = $server | Disconnect-DbaInstance
         }
     }
-}
+}