Skip to content

Commit 34d47df

Browse files
Restore-DbaDatabase - Fix ErrorBrokerConversations restore handling (review of #10253)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent efdc26e commit 34d47df

5 files changed

Lines changed: 203 additions & 14 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ Find real bugs (logic errors, null refs, incorrect behavior) and fix them. Skip
7474
| 232395207 | Set-DbaPrivilege, Get-DbaPrivilege - Add CreateGlobalObjects privilege support (#10235) | DONE | Fixed empty privilege-entry updates so Set-DbaPrivilege still grants rights when secedit exports a blank line; added unit regression test. |
7575
| 099624061 | Get-DbaDbRestoreHistory - Add BackupStartDate, StopAt, and LastRestorePoint columns (#10249) | DONE | Fixed LastRestorePoint so StopAt is used whenever specified; added unit regression test. |
7676
| 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. |
77-
| 8218d327e | Restore-DbaDatabase, Invoke-DbaAdvancedRestore - Add ErrorBrokerConversations parameter (#10253) | PENDING | |
77+
| 8218d327e | Restore-DbaDatabase, Invoke-DbaAdvancedRestore - Add ErrorBrokerConversations parameter (#10253) | DONE | Fixed missing ExecuteAs script prefix and added NoRecovery/Standby validation; added regression tests. |
7878
| 9a4e4bacb | Connect-DbaInstance - Set NonPooledConnection on ServerConnection (#10260) | PENDING | |
7979
| a0ab78a66 | Get-DbaCmObject - Apply CimOperationTimeout to all CIM connections (#10252) | PENDING | |
8080
| fbdb47053 | Import-DbaXESessionTemplate - Add event_file target when TargetFilePath specified (#10250) | PENDING | |

public/Invoke-DbaAdvancedRestore.ps1

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,14 @@ function Invoke-DbaAdvancedRestore {
262262
Stop-Function -Message "Failure" -Category ConnectionError -ErrorRecord $_ -Target $SqlInstance
263263
return
264264
}
265-
if ($KeepCDC -and ($NoRecovery -or ('' -ne $StandbyDirectory))) {
265+
if ($KeepCDC -and ($NoRecovery -or ("" -ne $StandbyDirectory))) {
266266
Stop-Function -Category InvalidArgument -Message "KeepCDC cannot be specified with Norecovery or Standby as it needs recovery to work"
267267
return
268268
}
269+
if ($ErrorBrokerConversations -and ($NoRecovery -or ("" -ne $StandbyDirectory))) {
270+
Stop-Function -Category InvalidArgument -Message "ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work"
271+
return
272+
}
269273

270274
if ($null -ne $PageRestore) {
271275
Write-Message -Message "Doing Page Recovery" -Level Verbose
@@ -424,15 +428,22 @@ function Invoke-DbaAdvancedRestore {
424428
if ($Pscmdlet.ShouldProcess($SqlInstance, "Restoring $database to $SqlInstance based on these files: $($backup.FullName -join ', ')")) {
425429
try {
426430
$restoreComplete = $true
431+
$executeAsLogin = $null
432+
if ($ExecuteAs -ne "" -and $BackupCnt -eq 1) {
433+
$executeAsLogin = $ExecuteAs.Replace("'", "''")
434+
}
427435
if (($KeepCDC -or $ErrorBrokerConversations) -and $restore.NoRecovery -eq $false) {
428436
$script = $restore.Script($server)
429437
$withOptions = @()
430-
if ($KeepCDC) { $withOptions += 'KEEP_CDC' }
431-
if ($ErrorBrokerConversations) { $withOptions += 'ERROR_BROKER_CONVERSATIONS' }
432-
if ($script -like '*WITH*') {
433-
$script = $script.TrimEnd() + ' , ' + ($withOptions -join ' , ')
438+
if ($KeepCDC) { $withOptions += "KEEP_CDC" }
439+
if ($ErrorBrokerConversations) { $withOptions += "ERROR_BROKER_CONVERSATIONS" }
440+
if ($script -like "*WITH*") {
441+
$script = $script.TrimEnd() + " , " + ($withOptions -join " , ")
434442
} else {
435-
$script = $script.TrimEnd() + ' WITH ' + ($withOptions -join ' , ')
443+
$script = $script.TrimEnd() + " WITH " + ($withOptions -join " , ")
444+
}
445+
if ($null -ne $executeAsLogin) {
446+
$script = "EXECUTE AS LOGIN='$executeAsLogin'; " + $script
436447
}
437448
if ($true -ne $OutputScriptOnly) {
438449
Write-Progress -id 1 -activity "Restoring $database to $SqlInstance - Backup $BackupCnt of $($Backups.count)" -percentcomplete 0 -status ([System.String]::Format("Progress: {0} %", 0))
@@ -449,8 +460,8 @@ function Invoke-DbaAdvancedRestore {
449460
}
450461
} elseif ($OutputScriptOnly) {
451462
$script = $restore.Script($server)
452-
if ($ExecuteAs -ne '' -and $BackupCnt -eq 1) {
453-
$script = "EXECUTE AS LOGIN='$ExecuteAs'; " + $script
463+
if ($null -ne $executeAsLogin) {
464+
$script = "EXECUTE AS LOGIN='$executeAsLogin'; " + $script
454465
}
455466
} elseif ($VerifyOnly) {
456467
Write-Message -Message "VerifyOnly restore" -Level Verbose
@@ -473,9 +484,9 @@ function Invoke-DbaAdvancedRestore {
473484
}
474485
Write-Progress -id 2 -ParentId 1 -Activity "Restore $($backup.FullName -Join ',')" -percentcomplete 0
475486
$script = $restore.Script($server)
476-
if ($ExecuteAs -ne '' -and $BackupCnt -eq 1) {
487+
if ($null -ne $executeAsLogin) {
477488
Write-Progress -id 1 -activity "Restoring $database to $SqlInstance - Backup $BackupCnt of $($Backups.count)" -percentcomplete 0 -status ([System.String]::Format("Progress: {0} %", 0))
478-
$script = "EXECUTE AS LOGIN='$ExecuteAs'; " + $script
489+
$script = "EXECUTE AS LOGIN='$executeAsLogin'; " + $script
479490
$null = $server.ConnectionContext.ExecuteNonQuery($script)
480491
Write-Progress -id 1 -activity "Restoring $database to $SqlInstance - Backup $BackupCnt of $($Backups.count)" -status "Complete" -Completed
481492
} else {

public/Restore-DbaDatabase.ps1

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,10 +634,14 @@ function Restore-DbaDatabase {
634634
return
635635
}
636636
}
637-
if ($KeepCDC -and ($NoRecovery -or ('' -ne $StandbyDirectory))) {
637+
if ($KeepCDC -and ($NoRecovery -or ("" -ne $StandbyDirectory))) {
638638
Stop-Function -Category InvalidArgument -Message "KeepCDC cannot be specified with Norecovery or Standby as it needs recovery to work"
639639
return
640640
}
641+
if ($ErrorBrokerConversations -and ($NoRecovery -or ("" -ne $StandbyDirectory))) {
642+
Stop-Function -Category InvalidArgument -Message "ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work"
643+
return
644+
}
641645
if ($Continue) {
642646
Write-Message -Message "Called with continue, so assume we have an existing db in norecovery"
643647
$WithReplace = $True

tests/Invoke-DbaAdvancedRestore.Tests.ps1

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0" }
22
param(
3-
$ModuleName = "dbatools",
3+
$ModuleName = "dbatools",
44
$CommandName = "Invoke-DbaAdvancedRestore",
55
$PSDefaultParameterValues = $TestConfig.Defaults
66
)
@@ -41,6 +41,117 @@ Describe $CommandName -Tag UnitTests {
4141
Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $hasParameters | Should -BeNullOrEmpty
4242
}
4343
}
44+
45+
InModuleScope dbatools {
46+
Context "ErrorBrokerConversations behavior" {
47+
BeforeAll {
48+
function Add-TeppCacheItem { }
49+
function New-MockRestore {
50+
$restore = [PSCustomObject]@{
51+
NoRecovery = $false
52+
StandbyFile = $null
53+
Database = $null
54+
ReplaceDatabase = $false
55+
MaxTransferSize = $null
56+
BufferCount = $null
57+
Blocksize = $null
58+
Checksum = $false
59+
Restart = $false
60+
KeepReplication = $false
61+
Action = $null
62+
FileNumber = $null
63+
ToPointInTime = $null
64+
StopBeforeMarkName = $null
65+
StopAtMarkName = $null
66+
StopBeforeMarkAfterDate = $null
67+
StopAtMarkAfterDate = $null
68+
RelocateFiles = (New-Object System.Collections.ArrayList)
69+
Devices = (New-Object System.Collections.ArrayList)
70+
}
71+
Add-Member -InputObject $restore -Name Script -MemberType ScriptMethod -Value {
72+
param($Server)
73+
"RESTORE DATABASE [$($this.Database)] FROM DISK = 'C:\backups\test.bak' WITH REPLACE"
74+
} -Force
75+
$restore
76+
}
77+
78+
$script:mockServer = [PSCustomObject]@{
79+
Databases = @()
80+
DatabaseEngineEdition = "SqlServer"
81+
ConnectionContext = [PSCustomObject]@{
82+
TrueLogin = "dbatoolsci"
83+
exists = $false
84+
}
85+
}
86+
Add-Member -InputObject $script:mockServer.ConnectionContext -Name ExecuteNonQuery -MemberType ScriptMethod -Value {
87+
param($Query)
88+
$null
89+
} -Force
90+
Add-Member -InputObject $script:mockServer.ConnectionContext -Name Disconnect -MemberType ScriptMethod -Value { } -Force
91+
92+
$script:backupHistory = [PSCustomObject]@{
93+
Database = "RestoreAsDb"
94+
Type = "1"
95+
FirstLsn = 1
96+
RestoreTime = (Get-Date).AddMinutes(-5)
97+
RecoveryModel = "Full"
98+
FileList = @(
99+
[PSCustomObject]@{
100+
LogicalName = "RestoreAsDb"
101+
PhysicalName = "C:\restore\RestoreAsDb.mdf"
102+
}
103+
)
104+
FullName = @("C:\backups\RestoreAsDb.bak")
105+
Position = 1
106+
}
107+
108+
function Test-FunctionInterrupt { $false }
109+
function Write-Message { }
110+
}
111+
112+
BeforeEach {
113+
Mock Connect-DbaInstance { $script:mockServer }
114+
Mock New-Object {
115+
New-MockRestore
116+
} -ParameterFilter {
117+
$TypeName -eq "Microsoft.SqlServer.Management.Smo.Restore"
118+
}
119+
Mock New-Object {
120+
[PSCustomObject]@{
121+
LogicalFileName = $null
122+
PhysicalFileName = $null
123+
}
124+
} -ParameterFilter {
125+
$TypeName -eq "Microsoft.SqlServer.Management.Smo.RelocateFile"
126+
}
127+
Mock New-Object {
128+
[PSCustomObject]@{
129+
Name = $null
130+
devicetype = $null
131+
}
132+
} -ParameterFilter {
133+
$TypeName -eq "Microsoft.SqlServer.Management.Smo.BackupDeviceItem"
134+
}
135+
}
136+
137+
It "Should call Stop-Function when ErrorBrokerConversations is combined with NoRecovery" {
138+
Mock Stop-Function {
139+
throw $Message
140+
}
141+
142+
{ Invoke-DbaAdvancedRestore -BackupHistory $script:backupHistory -SqlInstance "sql1" -NoRecovery -ErrorBrokerConversations } | Should -Throw "*ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work*"
143+
}
144+
145+
It "Should prefix OutputScriptOnly with Execute As when ErrorBrokerConversations is specified" {
146+
Mock Stop-Function { }
147+
$output = Invoke-DbaAdvancedRestore -BackupHistory $script:backupHistory -SqlInstance "sql1" -OutputScriptOnly -ErrorBrokerConversations -ExecuteAs "RestoreAs"
148+
$scriptOutput = $output | Select-Object -Last 1
149+
150+
$scriptOutput | Should -BeLike "EXECUTE AS LOGIN='RestoreAs'*ERROR_BROKER_CONVERSATIONS*"
151+
Should -Invoke Stop-Function -Times 0
152+
}
153+
}
154+
}
44155
}
45156
<#
46157
Integration test should appear below and are custom to the command you are writing.

tests/Restore-DbaDatabase.Tests.ps1

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#Requires -Module @{ ModuleName="Pester"; ModuleVersion="5.0" }
22
param(
3-
$ModuleName = "dbatools",
3+
$ModuleName = "dbatools",
44
$CommandName = "Restore-DbaDatabase",
55
$PSDefaultParameterValues = $TestConfig.Defaults
66
)
@@ -70,6 +70,34 @@ Describe $CommandName -Tag UnitTests {
7070
Compare-Object -ReferenceObject $expectedParameters -DifferenceObject $hasParameters | Should -BeNullOrEmpty
7171
}
7272
}
73+
74+
InModuleScope dbatools {
75+
Context "ErrorBrokerConversations validation" {
76+
BeforeAll {
77+
function Write-Message { }
78+
}
79+
80+
BeforeEach {
81+
Mock Connect-DbaInstance {
82+
[PSCustomObject]@{
83+
DatabaseEngineEdition = "SqlServer"
84+
VersionMajor = 16
85+
ConnectionContext = [PSCustomObject]@{
86+
StatementTimeout = 0
87+
}
88+
}
89+
}
90+
}
91+
92+
It "Should call Stop-Function when ErrorBrokerConversations is combined with NoRecovery" {
93+
Mock Stop-Function {
94+
throw $Message
95+
}
96+
97+
{ Restore-DbaDatabase -SqlInstance "sql1" -Path "C:\backups\test.bak" -ErrorBrokerConversations -NoRecovery } | Should -Throw "*ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work*"
98+
}
99+
}
100+
}
73101
}
74102

75103
Describe $CommandName -Tag IntegrationTests {
@@ -723,6 +751,41 @@ Describe $CommandName -Tag IntegrationTests {
723751
}
724752

725753

754+
Context "Checking ErrorBrokerConversations parameter " {
755+
BeforeAll {
756+
$databaseName = "testErrorBrokerConversations"
757+
$server = Connect-DbaInstance -SqlInstance $TestConfig.InstanceSingle -SqlCredential $TestConfig.SqlCredential -EnableException
758+
$executeAsLogin = $server.ConnectionContext.TrueLogin
759+
$escapedExecuteAsLogin = $executeAsLogin.Replace("'", "''")
760+
$server.ConnectionContext.Disconnect()
761+
}
762+
763+
It "Should have ERROR_BROKER_CONVERSATIONS in the SQL" {
764+
$output = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Path "$($TestConfig.appveyorlabrepo)\singlerestore\singlerestore.bak" -DatabaseName $databaseName -OutputScriptOnly -ErrorBrokerConversations -WithReplace
765+
$output | Should -BeLike "*ERROR_BROKER_CONVERSATIONS*"
766+
}
767+
768+
It "Should prefix the script with the Execute As statement when ErrorBrokerConversations is specified" {
769+
$output = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Path "$($TestConfig.appveyorlabrepo)\singlerestore\singlerestore.bak" -DatabaseName $databaseName -OutputScriptOnly -ErrorBrokerConversations -WithReplace -ExecuteAs $executeAsLogin
770+
$output | Should -BeLike "EXECUTE AS LOGIN='$escapedExecuteAsLogin'*ERROR_BROKER_CONVERSATIONS*"
771+
}
772+
773+
It "Should not output, and warn if Norecovery and ErrorBrokerConversations specified" {
774+
$WarnVar = $null
775+
$output = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Path "$($TestConfig.appveyorlabrepo)\singlerestore\singlerestore.bak" -DatabaseName $databaseName -OutputScriptOnly -ErrorBrokerConversations -WithReplace -NoRecovery -WarningAction SilentlyContinue
776+
$WarnVar | Should -BeLike "*ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work"
777+
$output | Should -Be $null
778+
}
779+
780+
It "Should not output, and warn if StandbyDirectory and ErrorBrokerConversations specified" {
781+
$WarnVar = $null
782+
$output = Restore-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -Path "$($TestConfig.appveyorlabrepo)\singlerestore\singlerestore.bak" -DatabaseName $databaseName -OutputScriptOnly -ErrorBrokerConversations -WithReplace -StandbyDirectory $backupPath -WarningAction SilentlyContinue
783+
$WarnVar | Should -BeLike "*ErrorBrokerConversations cannot be specified with Norecovery or Standby as it needs recovery to work"
784+
$output | Should -Be $null
785+
}
786+
}
787+
788+
726789
Context "Page level restores" {
727790
BeforeAll {
728791
$null = Get-DbaDatabase -SqlInstance $TestConfig.InstanceSingle -ExcludeSystem | Remove-DbaDatabase

0 commit comments

Comments
 (0)